Skip to content

Measure a tmux socket path where tmux resolves it - #730

Open
tony wants to merge 3 commits into
fix/723-repr-socket-dirfrom
fix/725-socket-path-length
Open

Measure a tmux socket path where tmux resolves it#730
tony wants to merge 3 commits into
fix/723-repr-socket-dirfrom
fix/725-socket-path-length

Conversation

@tony

@tony tony commented Jul 30, 2026

Copy link
Copy Markdown
Member

Fixes #725. Stacked on #727.

A tmux socket is a UNIX domain socket, so its path is capped by sockaddr_un — 107 bytes on Linux, 103 on macOS. Over that, tmux says:

error connecting to /tmp/dddd…/tmux-1000/default (File name too long)

That names the path, but not how far over the limit it is, nor which variable made it that long. This adds exc.SocketPathTooLong carrying all three.

Where the measurement happens

The routes are not equivalent, so they are not measured in the same place.

socket_path= is handed to tmux unchanged as -S<path>. Nothing can revise it, so it is measured at construction, where the caller still holds the frame that wrote it.

socket_name= is not. libtmux passes -L<name> and tmux resolves the directory from $TMUX_TMPDIR when it runs. Measured on tmux 3.7b: construct a Server(socket_name=…) with TMUX_TMPDIR=a, change it to b, then dispatch — tmux binds under b. So a construction-time check on that route tests a value with no bearing on what tmux will use. It passes when the variable later grows (hitting the exact File name too long this exists to prevent) and refuses when it later shrinks. Those paths are measured per command, at the same point colors is already validated.

A bare Server() inside a tmux pane is left alone entirely. A bare tmux client prefers $TMUX over $TMUX_TMPDIR, so a deep socket directory is irrelevant to it — measured: with $TMUX set, tmux list-sessions succeeds while $TMUX_TMPDIR names a directory far too deep to bind. Checking it would have refused a server tmux reaches without difficulty, in the case libtmux is most often used from.

Server.socket_args()

The -S/-L flags were being reconstructed in four places — Server.cmd(), Server.raise_if_dead(), the format-query layer, and the control-mode client, the last two spawning tmux through their own Popen/check_call. A check added to any one of them would have covered part of the dispatch surface. They now all build their argv from Server.socket_args(), so the measurement is total by construction rather than by discipline.

Constructing a server has no side effect

Naming a server is not using one. A named or bare Server always builds, so Server.is_alive() keeps answering for a server that cannot be reached — an address the kernel cannot hold is one more way of not being alive — and Server.raise_if_dead() keeps being the way to ask why. Server.sessions stays lenient and returns an empty QueryList, matching the documented contract for list accessors.

Verification

ruff check, ruff format, mypy (69 files), py.test --reruns 0 (1477 passed, 1 skipped), just build-docs — all clean. Green on tmux 3.2a through master.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.72093% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.63%. Comparing base (6ca471e) to head (b11c74f).

Files with missing lines Patch % Lines
src/libtmux/_internal/env.py 55.55% 4 Missing ⚠️
src/libtmux/exc.py 87.50% 2 Missing ⚠️
src/libtmux/server.py 94.11% 1 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                     @@
##           fix/723-repr-socket-dir     #730      +/-   ##
===========================================================
+ Coverage                    52.40%   52.63%   +0.22%     
===========================================================
  Files                           26       26              
  Lines                         3742     3764      +22     
  Branches                       747      747              
===========================================================
+ Hits                          1961     1981      +20     
- Misses                        1477     1481       +4     
+ Partials                       304      302       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony

tony commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. The socket_name branch validates a socket path libtmux computes, but tmux resolves symlinks before binding, so a configuration tmux handles fine is now refused at construction with no opt-out.

if socket_path is not None:
check_socket_path_length(socket_path)
self.socket_path = socket_path
else:
if socket_name is None and socket_name_factory is not None:
socket_name = socket_name_factory()
# Also covers the bare ``Server()``, whose socket path is inherited
# from ``$TMUX_TMPDIR`` in full.
check_socket_path_length(
resolve_socket_path(socket_name),
socket_name=socket_name,
)
if socket_name is not None:
self.socket_name = socket_name

Reproduced on this head, with TMUX_TMPDIR set to a 93-char symlink pointing at a short directory:

$ TMUX_TMPDIR=$LONG tmux -f /dev/null display-message -p '#{socket_path}'
/tmp/sd3394337/tmux-1000/default

tmux realpath()s the socket directory in make_label(), so the real path is 34 bytes and binds without complaint. libtmux's resolve_socket_path() does not, so it measures the unresolved 93-char symlink instead:

SocketPathTooLong: Socket path for socket_name='probe' is 109 bytes, over the 107 byte limit

decee4bf removed this same computation from Server.__init__ and gave the reason in its body — "Guessing the path would have been wrong anyway. tmux resolves a -L name against its own socket directory, which honours TMUX_TMPDIR, the per-uid directory and the platform's overrides. Callers that need the real path ask tmux for it" — and pinned the contract with test_socket_path_not_derived_from_socket_name. #727 reintroduced the guess but kept it harmless: repr text and a best-effort unlink, with a docstring repeating the caveat that the path is "computed, not observed". This PR turns the same guess into a hard failure.

The socket_path is not None branch is sound, since tmux uses -S verbatim with no realpath(). Only the resolved-from-socket_name branch inherits the guess. Options: os.path.realpath() the socket directory before measuring, or keep the eager check for the caller-supplied socket_path and defer the resolved-name case to where the path meets tmux.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the fix/723-repr-socket-dir branch from 8f437fa to 1bd85e1 Compare July 31, 2026 00:04
@tony
tony force-pushed the fix/725-socket-path-length branch 2 times, most recently from bd730e5 to 593d5ff Compare August 1, 2026 00:01
@tony tony changed the title Measure the socket path before tmux sees it Measure a tmux socket path where tmux resolves it Aug 1, 2026
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Force-pushed a redesign of where the check runs. The earlier revision validated the socket path in Server.__init__ on every route; two measurements against tmux 3.7b showed that is unsound for the routes that resolve through the environment.

tmux reads $TMUX_TMPDIR at exec, not at construction. Constructing Server(socket_name=N) with TMUX_TMPDIR=a, changing it to b, then dispatching, binds under b. A construction-time check on that route therefore tests a value with no bearing on what tmux uses — it passes when the variable later grows, which is the exact File name too long this issue is about, and refuses when it later shrinks.

A bare client prefers $TMUX over $TMUX_TMPDIR. With $TMUX set, tmux list-sessions succeeds while $TMUX_TMPDIR names a directory far too deep to bind. The previous revision refused to construct a bare Server() there — in the case libtmux is most often used from, a script running inside tmux.

socket_path= is unaffected and is still refused at construction, since libtmux passes it through unchanged as -S<path>.

This also closes the design question raised in review: Server.is_alive() stays total, because a named or bare Server always constructs.

@tony
tony force-pushed the fix/725-socket-path-length branch from 593d5ff to 337c921 Compare August 1, 2026 00:37
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue, now fixed in 337c921:

  1. test_bare_server_inside_a_pane_ignores_tmux_tmpdir patched $TMUX_TMPDIR for the whole test, leaking a live tmux daemon on every run. The server fixture takes monkeypatch, so pytest undoes the patches only after that fixture's finalizer has run — _reap_test_server then resolves the socket under the 120-char directory, cannot reach the server so skips kill(), and unlinks a path that never existed. Both halves of the reap contract fail silently while the test passes.

https://github.com/tmux-python/libtmux/blob/593d5ffd2ba4a1a0e9ac0e1fa06ba4a6e40b93cd/tests/test_server.py#L323-L332

Reproduced deterministically on that revision: the test passes, then tmux -L libtmux_test<...> list-sessions lists two live sessions. This is the same failure mode raised on the parent PR in this stack (#727), and it defeats the invariant _reap_test_server was written for (#660, stale sockets accumulating indefinitely).

Fixed by scoping the patches to the assertions with monkeypatch.context(), matching how #727 resolved it. Verified: no sockets left behind after the test, and none after the full tests/test_server.py run.

Also corrected in the same push: Server.socket_args()'s docstring said the check "belongs at dispatch rather than at construction", which stopped being accurate once an explicit socket_path became a construction-time check. It now states the split.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the fix/725-socket-path-length branch 2 times, most recently from c8356b9 to 66ff46f Compare August 1, 2026 03:12
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 3 issues, all fixed in 66ff46f:

  1. libtmux refused servers tmux reaches. tmux creates <dir>/tmux-<euid> and silently binds under /tmp when it cannot, so a $TMUX_TMPDIR that does not exist never holds the socket however long its name is. Measuring it anyway made is_alive() report a live daemon dead and the first command raise.

https://github.com/tmux-python/libtmux/blob/c8356b9d0e5e3f9b4e9c9e2f0e4f3d2a1b6c7e8f/src/libtmux/_internal/env.py#L143-L149

Measured: TMUX_TMPDIR=/tmp/<200 d's> tmux -L probe new-session -d exits 0 with the socket at /tmp/tmux-1000/probe, while libtmux reported is_alive() -> False and raised SocketPathTooLong. resolve_socket_path() now mirrors the fallback; a directory that exists and is too deep still raises.

  1. __repr__ named a socket the object does not use. It resolved from $TMUX_TMPDIR even inside a pane, where a bare client uses $TMUX — the rule this PR itself establishes for socket_args().

  2. Three tests and a doctest pinned the false refusal by pointing $TMUX_TMPDIR at a nonexistent directory. They now use a real one, so they still exercise the true positive.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the fix/725-socket-path-length branch 2 times, most recently from 4ad7f9c to 57d931d Compare August 1, 2026 04:08
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 2 issues, fixed in 57d931d:

  1. The previous round's fix was only half applied. Server.is_alive's doctest and the pytest-plugin usage page still pointed $TMUX_TMPDIR at a directory that does not exist, so after the /tmp fallback landed they stopped reaching the length check at all — they passed for the wrong reason.

https://github.com/tmux-python/libtmux/blob/4ad7f9c7e6b3c9a1f0d2e8b7a4c5d6e9f0a1b2c3/src/libtmux/server.py#L367-L372

Both now create the directory first, so the resolved path is one tmux would really bind.

  1. The Server class docstring built its directory with tempfile.mkdtemp(), leaking one per run. It now uses the tmp_path fixture through request, which pytest rotates.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the fix/725-socket-path-length branch from 57d931d to adcc295 Compare August 1, 2026 10:24
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 2 issues, fixed in adcc295:

  1. The stated tmux mechanism was wrong in four places. "tmux creates <dir>/tmux-<euid> and quietly falls back to /tmp when it cannot" describes the uncreatable case, which is a hard error rather than a fallback. Measured on tmux 3.7b: a nonexistent directory and a broken symlink both fall back and exit 0, while a directory that exists but is unwritable, or is a regular file, both fail with couldn't create directory. tmux takes the first of $TMUX_TMPDIR and /tmp that resolves.

https://github.com/tmux-python/libtmux/blob/57d931d0e0c9b0a0f8d6e2c1a3b5d7f9e0a2c4b6/src/libtmux/_internal/env.py#L111-L116

  1. is_dir() was the wrong predicate for that rule. exists() follows symlinks the way realpath() does, so a broken link falls back as tmux does, and a regular file is left to tmux to reject rather than silently resolving to a /tmp path tmux would never bind. Verified both the original regression and the true positive still behave correctly.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the fix/723-repr-socket-dir branch from 1bd85e1 to d368b44 Compare August 1, 2026 10:41
@tony
tony force-pushed the fix/725-socket-path-length branch 2 times, most recently from e07a79f to c24f1b3 Compare August 1, 2026 10:54
@tony
tony force-pushed the fix/723-repr-socket-dir branch from d368b44 to 77971b6 Compare August 1, 2026 11:21
@tony
tony force-pushed the fix/725-socket-path-length branch from c24f1b3 to 1ffaa55 Compare August 1, 2026 11:25
@tony
tony force-pushed the fix/723-repr-socket-dir branch from 77971b6 to 6ca471e Compare August 1, 2026 11:41
tony added 3 commits August 1, 2026 06:44
why: A tmux socket is a UNIX domain socket, so its path is capped by
`sockaddr_un` -- 107 bytes on Linux, 103 on macOS. tmux reports an
overrun as `error connecting to <path> (File name too long)`, which
names the path but not how far over it is, nor which variable made it
that long. The overrun is usually inherited rather than typed: a deep
pytest `tmp_path`, an XDG runtime dir, a nested worktree.

what:
- Add `exc.SocketPathTooLong` carrying the path, the byte count, how
  far over the limit it is, and the environment variable it came from.
- Measure each route where tmux itself resolves it. A `socket_path` is
  passed through unchanged as `-S<path>`, so it is settled at
  construction and refused there. A `socket_name` resolves against
  `$TMUX_TMPDIR`, which tmux re-reads at exec rather than when the
  Server was built, so measuring it at construction would check a value
  with no bearing on what tmux binds -- it is measured per command
  instead, where `colors` is already checked.
- Add `Server.socket_args()` and build every tmux spawn from it. The
  flags were previously reconstructed in four places -- `cmd`,
  `raise_if_dead`, the format-query layer and the control-mode client,
  the last two spawning tmux themselves -- so a check on any one of
  them would have covered part of the surface.
- Resolve a bare server's socket the way a bare tmux client does,
  preferring `$TMUX` over `$TMUX_TMPDIR`. Inside a pane tmux never
  consults the directory, so measuring it would refuse a server tmux
  reaches without difficulty.
- Constructing a named or bare Server has no side effect, so
  `is_alive()` still answers for a server that cannot be reached, and
  `raise_if_dead()` still reports why.

Fixes #725
why: A pytest tmp_path is nested deep by design, so a socket under it can
overrun what a UNIX socket address holds. The failure arrived as a tmux
error attributed to whichever command ran first.

what:
- Document the limit and the short-socket-directory workaround on the
  pytest-plugin usage page.
- Say on the configuration page that libtmux reads $TMUX_TMPDIR to know
  where the socket lands, and that a deep one raises SocketPathTooLong.
why: A socket path that cannot bind fails as a tmux error naming the
path but not the numbers, and a deep pytest `tmp_path` reaches the
limit without anyone typing a long path.

what:
- Record the socket-path measurement under `What's new`, naming where
  each route is measured and the shorter-directory workaround.
@tony
tony force-pushed the fix/725-socket-path-length branch from 1ffaa55 to b11c74f Compare August 1, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant